Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add isolated and simplified path to add components #1204

Merged
merged 16 commits into from
Oct 23, 2023

Conversation

EmosewaMC
Copy link
Collaborator

Follows a structure similar to std::thread in that you say what component you want to add in the Template types, and what you want to pass to the constructor in the rest of the variadic template values. This is a catch all solution and due to forwarding, is basically a free function call idenitcal to the old ones, but now all the newing and inserting is done in one place! Even replacing the component with a new one is all done at once.

before:

auto* newComp = new DerivedComp(this, arg1, arg2, arg3);
components.insert(pair<DerivedComp::ComponentType, newComp);

This had to be repeated for every component and is prone to error as well as weak to large changes (as you'll see in future changes)
after:

AddComponent<DerivedComp>(arg1, arg2, arg3);

since the component always needs to know its parent Entity, we always default the first argument to being the Entity and dont make the user specify this. Then the other 3 arguments have their template types deduced and the component is also added to the map for you, all in 1 step. If you try to add a component again, instead of deleting the previous one, we simply use a placement new to basically reset the current pointer to its constructed state. This allows us to skip a delete and free and keeps the head code path at minimal branches (1 in our code and the normal amount in the map insert anyways).

A small test for the placement new is attached as well.

Tested that I could load into Venture Explorer, Forbidden Valley Race, Cannon Cove Shooting Gallery and Crux Prime and that the tests pass.

Prevents the next commits ODR violation
inline script component ComponentType
Allows for much easier adding of components and is error proof by not allowing the user to add more than 1 of a specific component type to an Entity.
Move all to the new variadic templates AddComponent function to reduce clutter and ways the component map is modified.
The new function makes no assumptions.  Component is assumed to not exist and is checked for with operator[].  This will construct a null component which will then be newed if the component didnt exist, or it will just get the current component if it does already exist.  No new component will be allocated or constructed if the component already exists and the already existing pointer is returned instead.
For the case where the component may already exist, use a placement new to construct the component again, it would be constructed again, but would not need to go through the allocator.
aronwk-aaron
aronwk-aaron previously approved these changes Sep 30, 2023
Jettford
Jettford previously approved these changes Oct 9, 2023
@aronwk-aaron aronwk-aaron dismissed stale reviews from Jettford and themself via ef27ead October 9, 2023 20:23
aronwk-aaron
aronwk-aaron previously approved these changes Oct 9, 2023
@EmosewaMC
Copy link
Collaborator Author

Need to add manual destructor call to placement newd component.

aronwk-aaron
aronwk-aaron previously approved these changes Oct 12, 2023
Update Entity.cpp
Copy link
Collaborator

@Jettford Jettford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You scare me.

@aronwk-aaron aronwk-aaron merged commit ae349d6 into main Oct 23, 2023
3 checks passed
@aronwk-aaron aronwk-aaron deleted the somethingElse branch October 23, 2023 01:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants